-
-
Notifications
You must be signed in to change notification settings - Fork 837
chore(helm): migrate to bitnami legacy registry and add configurable utility images #2574
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
Adds configurability for init and token syncer container images through new values in the Helm chart configuration
|
Walkthrough
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches🧪 Generate unit tests
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🧪 Early access (Sonnet 4.5): enabledWe are currently testing the Sonnet 4.5 model, which is expected to improve code review quality. However, this model may lead to increased noise levels in the review comments. Please disable the early access features if the noise level causes any inconvenience. Note:
Comment |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 2
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (7)
docker/dev-compose.yml
(1 hunks)docker/docker-compose.yml
(1 hunks)hosting/docker/webapp/docker-compose.yml
(2 hunks)hosting/k8s/helm/Chart.yaml
(1 hunks)hosting/k8s/helm/templates/_helpers.tpl
(1 hunks)hosting/k8s/helm/templates/webapp.yaml
(2 hunks)hosting/k8s/helm/values.yaml
(6 hunks)
🧰 Additional context used
🧠 Learnings (3)
📚 Learning: 2025-06-25T13:18:44.103Z
Learnt from: nicktrn
PR: triggerdotdev/trigger.dev#2195
File: hosting/k8s/helm/Chart.yaml:1-18
Timestamp: 2025-06-25T13:18:44.103Z
Learning: For the Trigger.dev Helm chart, the chart name should be "trigger" (not "trigger-v4") since this is the first official chart release. Helper templates should use the actual chart name from .Chart.Name rather than hardcoded prefixes.
Applied to files:
hosting/k8s/helm/Chart.yaml
📚 Learning: 2025-06-25T13:20:17.174Z
Learnt from: nicktrn
PR: triggerdotdev/trigger.dev#2195
File: hosting/k8s/helm/values.yaml:22-51
Timestamp: 2025-06-25T13:20:17.174Z
Learning: In the Trigger.dev Helm chart values.yaml, the maintainer prefers to use explicit comprehensive warnings for security-sensitive default values rather than implementing secure-by-default behavior that would fail installation. The project uses deterministic default secrets with clear "TESTING ONLY" warnings and instructions for production deployment.
Applied to files:
hosting/k8s/helm/Chart.yaml
📚 Learning: 2025-06-25T13:18:04.827Z
Learnt from: nicktrn
PR: triggerdotdev/trigger.dev#2195
File: hosting/k8s/helm/templates/extra-manifests.yaml:1-4
Timestamp: 2025-06-25T13:18:04.827Z
Learning: In the Trigger.dev v4 Helm chart, the user prefers to rely on documentation and examples in values files rather than implementing defensive coding in templates, particularly for features like extraManifests where proper usage is documented.
Applied to files:
hosting/k8s/helm/Chart.yaml
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (23)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (8, 8)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (7, 8)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (6, 8)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (5, 8)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (3, 8)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (2, 8)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (4, 8)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (1, 8)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (5, 8)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (4, 8)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (6, 8)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (2, 8)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (3, 8)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (7, 8)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (1, 8)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (8, 8)
- GitHub Check: units / packages / 🧪 Unit Tests: Packages (1, 1)
- GitHub Check: e2e / 🧪 CLI v3 tests (ubuntu-latest - npm)
- GitHub Check: e2e / 🧪 CLI v3 tests (windows-latest - npm)
- GitHub Check: e2e / 🧪 CLI v3 tests (ubuntu-latest - pnpm)
- GitHub Check: e2e / 🧪 CLI v3 tests (windows-latest - pnpm)
- GitHub Check: typecheck / typecheck
- GitHub Check: Analyze (javascript-typescript)
🔇 Additional comments (13)
docker/dev-compose.yml (1)
51-51
: LGTM! Consistent migration to legacy registry.The ClickHouse image has been correctly updated to use the
bitnamilegacy
registry, consistent with changes across other Docker Compose files in this PR.hosting/k8s/helm/Chart.yaml (1)
5-5
: LGTM! Appropriate version bump.The patch version increment from 4.0.2 to 4.0.3 correctly reflects the infrastructure changes (registry migration and image configurability enhancements) without breaking changes.
docker/docker-compose.yml (1)
79-79
: LGTM! Consistent with other compose file updates.The ClickHouse image correctly migrates to the
bitnamilegacy
registry, maintaining consistency with the dev environment configuration.hosting/k8s/helm/templates/webapp.yaml (2)
70-78
: LGTM! Improved configurability with dynamic image selection.The init container has been appropriately renamed from
init-shared
tovolume-permissions
(more descriptive), and now uses a template helper for the image reference. The addition ofimagePullPolicy
from values provides better control over image pulling behavior.
81-82
: LGTM! Template-driven image configuration added.The token-syncer container now uses a dynamic image reference via the helper template, replacing the hardcoded
bitnami/kubectl:1.28
. This enables proper configurability and supports the global image registry override pattern.hosting/docker/webapp/docker-compose.yml (2)
142-142
: LGTM! ClickHouse migrated to legacy registry.The ClickHouse image correctly switches to
bitnamilegacy
registry while maintaining the configurable tag pattern viaCLICKHOUSE_IMAGE_TAG
environment variable.
186-186
: LGTM! MinIO migrated to legacy registry.The MinIO image correctly switches to
bitnamilegacy
registry while preserving the configurable tag pattern viaMINIO_IMAGE_TAG
environment variable.hosting/k8s/helm/values.yaml (4)
383-386
: LGTM: PostgreSQL image migration to legacy registry.The migration to
bitnamilegacy/postgresql
is correctly configured. The commented image reference provides helpful documentation.
438-441
: LGTM: Redis image migration to legacy registry.The migration to
bitnamilegacy/redis
follows the same pattern as PostgreSQL and is correctly configured.
533-536
: LGTM: ClickHouse image migration to legacy registry.The migration to
bitnamilegacy/clickhouse
is correctly configured and consistent with other subchart migrations.
600-603
: No issue – 2025.6.13 is a valid tag.Bitnami’s legacy repository currently lists MinIO tags up through 2025.7.23, so the commented 2025.6.13 reference is correct.
Likely an incorrect or invalid review comment.
hosting/k8s/helm/templates/_helpers.tpl (2)
98-110
: LGTM: volumePermissions image helper follows established patterns.The helper template correctly implements registry override logic consistent with existing image helpers in the file.
112-124
: LGTM: tokenSyncer image helper follows established patterns.The helper template correctly implements registry override logic consistent with the volumePermissions helper and other image helpers in the file.
…utility images (#2574) * chore(docker): use bitnami legacy repo * chore(helm): use bitnami legacy repo * Make Helm webapp chart images configurable Adds configurability for init and token syncer container images through new values in the Helm chart configuration * chore(helm): refactor utility image config * chore(helm): bump chart version to 4.0.3 --------- Co-authored-by: LeoKaynan <leokaynan@hotmail.com>
Migrates all Bitnami chart dependencies to the bitnamilegacy registry following Bitnami's repository restructuring, and adds full configurability for webapp utility container images using Bitnami-style configuration patterns. Users can now customize the busybox and kubectl images used by the webapp pod, enabling compatibility with private registries and specific image versions.
Backwards Compatibility
The configuration structure has been changed but maintains the same default images and behavior.
Changes Made
Bitnami Legacy Registry Migration
bitnamilegacy/
image prefix for PostgreSQL and Redisbitnamilegacy/
repositorybitnamilegacy/postgresql:17.5.0-debian-12-r12
bitnamilegacy/redis:8.0.2-debian-12-r4
bitnamilegacy/clickhouse:25.6.1-debian-12-r0
bitnamilegacy/minio:2025.6.13-debian-12-r0
Utility Image Configurability
values.yaml
to usewebapp.volumePermissions.image
andwebapp.tokenSyncer.image
sections following Bitnami conventionsinit-shared
tovolume-permissions
to match Bitnami standardstrigger-v4.webapp.volumePermissions.image
andtrigger-v4.webapp.tokenSyncer.image
in_helpers.tpl
webapp.yaml
template to use new helper functionsglobal.imageRegistry
Utility Image Config
Testing